characterization tests and atime overhead guard for [ modularization - #7931

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.09%. Comparing base (fe94435) to head (2f7421f).

@@           Coverage Diff           @@
##           master    #7931   +/-   ##
=======================================
  Coverage   99.08%   99.09%           
=======================================
  Files          88       88           
  Lines       17429    17429           
=======================================
+ Hits        17270    17271    +1     
+ Misses        159      158    -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

Aside -- are you using gh to manage the stack? Curious how the experience is. I still swear by jj but it doesn't play that nicely with managing the stack on GitHub -- it's trivial to manage the stack locally with jj, but conveying that to GitHub, less so.

Aside -- are you using gh to manage the stack? Curious how the experience is. I still swear by jj but it doesn't play that nicely with managing the stack on GitHub -- it's trivial to manage the stack locally with jj, but conveying that to GitHub, less so.

Nah I'm doing it quite old school. I have a development branch where I do everything and push a lot locally. Before pushing to github I cherry-pick and squash manually.

well consider this my shameless plug for jj, which I've been using exclusively over git for a while now :)

you guys don’t use magit ??!! (emacs joke)

Error on CI is

Error in eval(bysub, xss, parent.frame()) : 
  numeric 'envir' arg not of length one
Calls: <Anonymous> ... system.time -> do.call -> <Anonymous> -> run_bench_mark

To reproduce locally I did

tinfo <- atime::atime_pkg_test_info("~/R/data.table")
tcall <- tinfo$test.call[["DT[i,j,by] per-call overhead for #852"]]
options(datatable.verbose=TRUE)
tres <- eval(tcall)

and I got

i clause present and columns used in by detected, only these subset: [g]
cedta decided 'data.table.7b7f63b04693991e014e245c8d4a0bc6b9dc38c6' wasn't data.table aware. Here is call stack with [[1L]] applied:
[[1]]
eval

[[2]]
eval

[[3]]
atime::atime_versions
…
[[7]]
run_bench_mark

[[8]]
suppressWarnings

[[9]]
withCallingHandlers

[[10]]
eval

[[11]]
eval

[[12]]
bench::mark

[[13]]
eval_one

[[14]]
eval

[[15]]
eval

[[16]]
data.table.7b7f63b04693991e014e245c8d4a0bc6b9dc38c6:::`[.data.table`

[[17]]
`[.data.table`

[[18]]
cedta

Error in eval(bysub, xss, parent.frame()) : 
  numeric 'envir' arg not of length one

This is happening when i is used at same time as by: (the issue does not happen in the other two usages in the test case, because they do not use i with by)

data.table:::`[.data.table`(dt, a > 2L, .(s = sum(v)), by = g)

which calls this code in [.data.table:

            xss = `[.data.table`(x,irows,allbyvars,with=FALSE,nomatch=nomatch,mult=mult,roll=roll,rollends=rollends)
          } else {
            if (verbose) catf("i clause present but columns used in by not detected. Having to subset all columns before evaluating 'by': '%s'\n", deparse(by))
            xss = `[.data.table`(x,irows,nomatch=nomatch,mult=mult,roll=roll,rol

I think the issue is that cedta thinks these recursive calls are not dt-aware? Not sure how to fix.

I think the issue is that cedta thinks these recursive calls are not dt-aware? Not sure how to fix.

Yes. I hope my fix works, fingers crossed :D

• HEAD=brk/a1_tests much faster for Date-IDate regression in #7213

(Plot image upload to picrd.com failed; download the artifact below to view plots)

Generated via commit 2f7421f

Download link for the artifact containing the test results: ↓ atime-results.zip

BTW, feel free to submit Approved PRs in any order you'd like by reordering the stack. Another thing I find jj excels at 😉

BTW, feel free to submit Approved PRs in any order you'd like by reordering the stack. Another thing I find jj excels at 😉

Copy that. I have added an integration branch at the bottom, so we can merge the PRs, one-by-one and do then one final merge of the integration branch